Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #195 by implementing topological sorting for PostgreSQL type creation to ensure types are created in the correct dependency order. Previously, types were sorted alphabetically (with domains last), which could fail when composite types reference other custom types like enums.
Key changes:
- Replaced simple alphabetical sorting with dependency-aware topological sorting using Kahn's algorithm
- Added support for detecting type-to-type dependencies in composite types and domain types
- Added test case demonstrating correct ordering of enum and composite types with dependencies
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/diff/type.go | Replaced alphabetical sorting logic with call to new topologicallySortTypes function |
| internal/diff/topological.go | Added topologicallySortTypes, extractTypeName, and findLastDot functions to implement dependency-aware type sorting |
| testdata/diff/dependency/type_to_type/new.sql | Test input with composite type referencing enum type |
| testdata/diff/dependency/type_to_type/old.sql | Empty schema baseline for test |
| testdata/diff/dependency/type_to_type/plan.sql | Expected SQL output with correct type ordering (enum before composite) |
| testdata/diff/dependency/type_to_type/plan.txt | Human-readable plan showing the expected migration steps |
| testdata/diff/dependency/type_to_type/plan.json | JSON format of the migration plan |
| testdata/diff/dependency/type_to_type/diff.sql | Generated diff SQL matching expected plan |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* Initial plan * Add detailed cycle-breaking comments to topologicallySortTypes Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> * Fix line reference in topologicallySortTypes comment Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com>
* Initial plan * Add comprehensive unit tests for topologicallySortTypes function Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> * Remove duplicate newTestCompositeTypeMultiple helper function Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* fix: type topological sort * Add detailed cycle-breaking comments to topologicallySortTypes (pgplex#206) * Initial plan * Add detailed cycle-breaking comments to topologicallySortTypes Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> * Fix line reference in topologicallySortTypes comment Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> * Add unit tests for topologicallySortTypes function (pgplex#207) * Initial plan * Add comprehensive unit tests for topologicallySortTypes function Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> * Remove duplicate newTestCompositeTypeMultiple helper function Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianzhou <230323+tianzhou@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Fix #195